Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix saved media url starting with https::// #15

Merged
merged 1 commit into from
May 6, 2022

Conversation

silviodaminato
Copy link
Contributor

I noticed that the url of the uploaded media was saved as https:://bucket.s3.region.amazonaws.com/path/object_key (i.e.: with double colon after the url schema).
I added a check to see whether it's necessary or not.

@zoomoid
Copy link
Owner

zoomoid commented May 6, 2022

I really wonder why the resolved Endpoint's protocol would already contain the colon? The spec for URI schemes tells otherwise, but nonetheless thank you for your contribution regarding catching this behaviour. I will merge this as soon as I have a dev setup again.

Could you maybe also provide me with details in regards to your setup, i.e., S3 provider etc?

@zoomoid zoomoid self-requested a review May 6, 2022 16:02
@zoomoid
Copy link
Owner

zoomoid commented May 6, 2022

Nevermind, I dug into this myself and found this obscurity: https://github.com/aws/aws-sdk-js-v3/blob/03e589f6e1b760dc365b4ccb17f8523bab71e1fa/packages/protocol-http/src/httpRequest.ts#L24 from the Endpoint type documentation.

This being only discovered now can only be attributed to me not testing the case of not defining a CDN_BASE_URL and using the bucket url directly (after all, that's what this package is for).

Thank you for pointing this out, @silviodaminato!

@zoomoid zoomoid merged commit a15484f into zoomoid:main May 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants